Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

provider/aws: allow local kinesis #3255

Merged
merged 1 commit into from
Oct 29, 2015
Merged

provider/aws: allow local kinesis #3255

merged 1 commit into from
Oct 29, 2015

Conversation

garrettheel
Copy link

Allow providing a custom endpoint for kinesis so that Terraform can provision locally (via tools like kinesalite).

The one thing I'm still not happy with is that valid IAM credentials are required even when only provisioning locally due to the early ValidateCredentials call. This also affects dynamodb_endpoint. Suggestions on how to fix this are welcome.

Thanks!

apparentlymart
apparentlymart reviewed Sep 17, 2015
View reviewed changes
builtin/providers/aws/config.go
@@ -121,8 +118,11 @@ func (c *Config) Client() (interface{}, error) {
log.Println("[INFO] Initializing RDS Connection")
client.rdsconn = rds.New(awsConfig)

awsKinesisConfig := awsConfig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since awsConfig is a pointer I think you're just making a new reference to the same struct here and then mutating it, rather than copying it as I think you intended.

@garrettheel
Copy link
Author

@apparentlymart Thanks for the note on copying the pointer, I've since updated this

@garrettheel
Copy link
Author

Bump - anything you need from me to get this merged?

@catsby
Copy link
Contributor

catsby commented Oct 28, 2015

Can you help me understand the change to the DynamoDB connection?
It looks like the endpoint can no longer overwrite, it's not the only way.

Same for this Kinesis connection; if it's omitted, aws.String() will provide a pointer to "", which I imagine doesn't work

@catsby catsby added the waiting-response An issue/pull request is waiting for a response from the community label Oct 28, 2015
@garrettheel
Copy link
Author

@catsby the change to the DynamoDB connection is just to remove some of the boilerplate of redefining the entire config object. It takes a copy of awsConfig and then sets Endpoint to the provided value (which may be empty, mimicking existing behavior).

I've tested this by overriding KinesisEndpoint (pointing to a local kinesalite) and not overriding it and both work as expected.

@catsby
Copy link
Contributor

catsby commented Oct 29, 2015

OK, thanks for the explanation

catsby added a commit that referenced this pull request Oct 29, 2015
@catsby
Merge pull request #3255 from Runscope/local-kinesis
cb2ecf5 
provider/aws: allow local kinesis
@catsby catsby merged commit cb2ecf5 into hashicorp:master Oct 29, 2015
@garrettheel garrettheel deleted the local-kinesis branch October 29, 2015 17:08
@radeksimko radeksimko mentioned this pull request Dec 17, 2015
Closed
@ghost
Copy link

ghost commented Apr 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement provider/aws waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@garrettheel @catsby @apparentlymart @radeksimko